setuptools ANN201 autofixes for fully untyped functions #4711
Conversation
Avasam
changed the title
Avasam
force-pushed
the
setuptools-ANN201-autofixes
branch
2 times, most recently
from
59376d7
to
6d830ef
Compare
setuptools/config/setupcfg.py
Outdated
| @@ -280,7 +281,7 @@ def _section_options( | |||
| yield name.lstrip('.'), value | |||
|
|
|||
| @property | |||
| def parsers(self): | |||
| def parsers(self) -> NoReturn: | |||
There was a problem hiding this comment.
Ok, isn't this going to make more complicated inheritance and require some form of overload annotation later?
It is kind of an abstract method that needs to be implemented.
The expected signature is something like () -> dict[str, Callable[[str], Any]] or more specific1.
I suppose that for the sake of documentation/understanding that return value as a dict is better than NoReturn.
Footnotes
-
The return values for the callable are probably something like all the types that can be produced by
ast.parse_literal(or subtypes) +packaging.specifiers.SpecifierSet↩
There was a problem hiding this comment.
NoReturn is a bottom type, so as far as subclassing it shouldn't be an issue.
For consumers it means that if something returns a ConfigHandler, they'd see accessing the property parsers as never returning (unless they use isinstance to ensure a subtype or something).
So you are correct that
I suppose that for the sake of documentation/understanding that return value as a
dictis better thanNoReturn.
Even better, this property should be marked abstract, so type-checkers tells the dev to never instanciate ConfigHandler directly, which implies that any ConfigHandler the dev gets should be a concrete subclass, so returning NoReturn for a NotImplementedError is no longer relevant.
But that's for a separate PR ! (I'll just revert this line here)
Avasam
force-pushed
the
setuptools-ANN201-autofixes
branch
from
6d830ef
to
747ae28
Compare
Avasam
force-pushed
the
setuptools-ANN201-autofixes
branch
from
747ae28
to
04b515e
Compare
|
Thank you very much. |
Summary of changes
I commented
[lint.per-file-ignores] "setuptools/**" = ["ANN2"]and[lint.flake8-annotations] ignore-fully-untyped = trueinruff.tomlthen ranruff check setuptools --select=ANN201 --fix --unsafe-fixes.I removed the added return types that were revealing hidden mypy issues (often
initialize_options) to keep the changes here completely automated.This has some overlap with #4709, but they can be merged in either order.
Pull Request Checklist
newsfragments/. (no public facing changes)(See documentation for details)